Skip to content

Conversation

@nicholaspai
Copy link
Member

No description provided.

Copy link
Member

@chrismaree chrismaree left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks really good! where should we post the outcome of this? would adding a README file to the test/gas-analytics dir make sence to show the costs of some of the common function calls/actions and also outlining the implications for this stress test on the max leaf count?

Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

// This test should inform the limit # refunds that we would allow a RelayerRefundLeaf to contain to avoid
// publishing a leaf that is unexecutable due to the block gas limit.

// Regarding the block limit, we should target a maximum of 30 million gas. Technically block's can support up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these comments are a little confusing. It makes it sound like we're targeting 30 million. Maybe we should just say that the max limit is 30 million, the expected block gas limit is 15 million, and we use 12 million as a conservative upper-bound.

console.log(`executeRelayerRefundRoot-gasUsed: ${receipt.gasUsed}`);
});
it(`Stress Test: 1 leaf contains ${STRESS_TEST_REFUND_COUNT} refunds with amount > 0`, async function () {
// This test should inform the limit # refunds that we would allow a RelayerRefundLeaf to contain to avoid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any information on limits on most L2s? If you don't know, that's totally fine, we should just add a TODO to figure out a good limit that's below all L2s that we're deploying to.

// and likely an underestimate because we are relaying tokens via the MockAdapter, not an Adapter used for
// production.

// Regarding the block limit, we should target a maximum of 30 million gas. Technically block's can support up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the other file, I think this comment is a bit misleading because of the word target.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants